feat: Create actual indexes and fix up state + error logic CLOUDP-317945 #6933
Conversation
rubydong
changed the title
rubydong
added
the
no-title-validation
| let isShowSuggestionsButtonDisabled = !hasNewChanges; | ||
|
|
||
| // Validate query upon typing | ||
| try { | ||
| parseFilter(inputQuery); | ||
|
|
||
| if (!inputQuery.startsWith('{') || !inputQuery.endsWith('}')) { | ||
| isShowSuggestionsButtonDisabled = true; | ||
| } | ||
| } catch (e) { | ||
| isShowSuggestionsButtonDisabled = true; | ||
| } |
There was a problem hiding this comment.
Nit: parsing the query on every re-render seems a big too excessive, let's pack this logic inside a useMemo
| @@ -104,10 +106,13 @@ const QueryFlowSection = ({ | |||
| }: SuggestedIndexFetchedProps) => Promise<void>; | |||
| indexSuggestions: Record<string, number> | null; | |||
| fetchingSuggestionsState: IndexSuggestionState; | |||
| initialQuery: Document | null; | |||
| initialQuery: string | null; | |||
There was a problem hiding this comment.
This type is incorrect (as the JSON.stringify that you still keep below indicates), you're still getting a document here, not a string, why the change?
There was a problem hiding this comment.
you're right -- i was trying to fix the type but messed it up along the way. i fixed it!
| : 'Error parsing query. Please follow query structure.', | ||
| indexSuggestionsState: 'error', | ||
| }); | ||
| track('Error parsing query', { context: 'Create Index Modal' }); |
There was a problem hiding this comment.
Let's add TODOs that point to a ticket for cleaning up these non real tracking events, I think you mentioned that there is one that should be part of the project
There was a problem hiding this comment.
added TODOs and yes these will be cleaned up in wrap-up
|
(merged main to see if CI failures are unrelated to changes here) |
| @@ -343,7 +339,7 @@ function getInitialState(): State { | |||
|
|
|||
| //------- | |||
|
|
|||
| export const createIndexOpened = (query?: BsonDocument) => ({ | |||
| export const createIndexOpened = (query?: Document) => ({ | |||
There was a problem hiding this comment.
I see that both you and @waltertan12 added an import here from two different packages, but this points to the same type, so this change is not really doing anything and it seems confusing to have both. Can you leave just one, either Document or BsonDocument?
|
CI failures seem related to your changes as they are failing in the compass-indexes package, can you take a look? |
yes thanks for checking and pulling in master. i updated a test to fix this |
Description
Had to re-enable the button always to get this error to show up -- normally there's validation so user won't be able to click on the button in the first place with parsing errors
Also mocked this error by manipulating the # of rows. It is unlikely for the user to encounter this normally.
Drive-by fixes to open from nudge styling + lint
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes